Skip to content

Use OS temp dir in runString#330

Open
StantonMatt wants to merge 1 commit into
extrabacon:masterfrom
StantonMatt:fix-runstring-tempdir
Open

Use OS temp dir in runString#330
StantonMatt wants to merge 1 commit into
extrabacon:masterfrom
StantonMatt:fix-runstring-tempdir

Conversation

@StantonMatt
Copy link
Copy Markdown

Refreshes stale PR #320 with the same minimal fix and focused coverage.

PythonShell.runString() currently builds its temp file path with the imported tmpdir function reference instead of calling tmpdir(). That can produce an invalid path before PythonShell.run() is reached.

This change uses path.join(tmpdir(), ...) and adds a regression test that intercepts PythonShell.run() to verify:

  • the generated script path is in the OS temp directory
  • the generated file contains the provided code
  • the original run options are passed through unchanged

Verification:

  • npm test -- --grep "runString"
  • npm test
  • npx prettier --check index.ts test/test-python-shell.ts
  • git diff --check
  • review-fix-loop: no actionable regressions found

Signed-off-by: Matthew Stanton <stantonmatthewj@gmail.com>
@kikiminyes
Copy link
Copy Markdown

I checked this locally on Windows.

Validated:

  • npm run compileOnce passes
  • npx mocha -r ts-node/register test/test-python-shell.ts --grep "temporary file" passes

The fix looks correct to me: tmpdir is imported as a function from os, so the old tmpdir + sep + ... path construction would stringify the function instead of using the OS temp directory. The new join(tmpdir(), ...) path and the focused test cover that behavior.

I could not use the full npm test result as a signal in this local environment because the installed WindowsApps Python launcher fails to spawn here (spawn UNKNOWN), causing the Python-dependent baseline tests to fail independently of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants